Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

register_to_file: Support Sensitive regtoken #164

Merged
merged 4 commits into from
Jun 17, 2024
Merged

register_to_file: Support Sensitive regtoken #164

merged 4 commits into from
Jun 17, 2024

Conversation

arusso
Copy link
Contributor

@arusso arusso commented Mar 11, 2023

Pull Request (PR) description

When retrieving a runner registration token from a lookup plugin such as vault_lookup the token will be marked as Sensitive. This change allows for an object of Sensitive[String] type to be passed in for regtoken and ensures it is properly unwrapped before use.

@alexjfisher
Copy link
Member

Looks correct. Could you regenerate the REFERENCE.md (bundle exec rake strings:generate:reference)? I think that's what the test failure is complaining about.

@smortex
Copy link
Member

smortex commented Mar 16, 2023

Hum, maybe your bundle is outdated?

bundle update and then bundle exec rake strings:generate:reference?

@arusso
Copy link
Contributor Author

arusso commented Mar 16, 2023

No change. It seems like it's complaining about the @return missing from the unregister_from_file.rb. Am I reading that right?

It's not a file I touched, but maybe we just need to add an @return Any or modify the dispatch to be clear that a String is always returned?

@arusso arusso force-pushed the regtoken-unwrap branch 2 times, most recently from 19cd27e to d77ca1b Compare March 16, 2023 22:51
smortex
smortex reviewed Mar 16, 2023
View reviewed changes
Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks-like CI is happier 👍

Just a minor thing, and I think we are good to go!

lib/puppet/functions/gitlab_ci_runner/register_to_file.rb Outdated Show resolved Hide resolved
@arusso
Copy link
Contributor Author

arusso commented Mar 16, 2023

Looks-like CI is happier 👍

Just a minor thing, and I think we are good to go!

Ended up rebuilding my ruby environment and that seemed to do the trick.

smortex
smortex reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@smortex smortex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. Maybe a test that ensure we do not break this by error in the future would be great? #165 should have fixed CI, can you rebase on top of it?

@arusso arusso force-pushed the regtoken-unwrap branch from ec7c2d8 to 4bd6f4a Compare May 10, 2023 05:26
@arusso
Copy link
Contributor Author

arusso commented May 10, 2023

I took a stab at the spec test for this change and it passed when run locally. Not sure what to make of the automated tests, seems like they're broken (at least) due to Puppet 8.0.1 needing a newer version of Ruby than whats provided?

@arusso
Copy link
Contributor Author

arusso commented Nov 25, 2023

Anything I can/need do to get this merged, or is this still broken due to the Puppet 8 release?

@smortex
Copy link
Member

smortex commented Nov 25, 2023

I think the error was fixed. Let's close & re-open to trigger a build.

@smortex smortex closed this Nov 25, 2023
@smortex smortex reopened this Nov 25, 2023
@traylenator
Copy link
Contributor

Just ran into this - patch works great thanks.

@traylenator
Copy link
Contributor

Approved but given the age here a rebase makes sense I would say.

arusso and others added 4 commits April 24, 2024 10:31
@arusso
register_to_file: Support Sensitive regtoken
3d4288d 
When retrieving a runner registration token from a lookup plugin such as
`vault_lookup` the token will be marked as Sensitive. This change allows
for an object of `Sensitive[String]` type to be passed in for `regtoken`
and ensures it is properly unwrapped before use.
@arusso @smortex
Do not accept empty sensitive strings
be4a16c 
Co-authored-by: Romain Tartière <[email protected]>
@arusso
Regenerate reference documentation
86c1e99
@arusso
Add spec test for sensitive regtoken parameter
a521dfb
@traylenator
Copy link
Contributor

#191 should fix tests.

@traylenator traylenator added the enhancement New feature or request label May 1, 2024
@traylenator traylenator merged commit 71569e9 into voxpupuli:master Jun 17, 2024
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smortex smortex smortex left review comments

@traylenator traylenator traylenator approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arusso @alexjfisher @smortex @traylenator